feat: input midi velocity into a separate track #182
Conversation
| player.Process(buf, midiContext, trackerUi) | ||
| player.Process(buf, midiContext) |
There was a problem hiding this comment.
this third argument was a relic from my old solution, which is why you introduced the broker. ergo, I got rid of it
| func (m *Model) CurrentPlayerConstraints() PlayerProcessConstraints { | ||
| d, ok := m.currentDerivedForTrack() | ||
| if !ok { | ||
| return PlayerProcessConstraints{IsConstrained: false} | ||
| } | ||
| return PlayerProcessConstraints{ | ||
| IsConstrained: m.trackMidiIn, | ||
| MaxPolyphony: len(d.tracksWithSameInstrument), | ||
| InstrumentIndex: d.instrumentRange[0], | ||
| } | ||
| } |
There was a problem hiding this comment.
PlayerProcessConstraints are a new concept - depending on what some settings in the Model are, the Player, via the PlayerProcessContext, might need to take care of some constraining conditions
|
|
||
| MenuClickable struct { | ||
| Clickable Clickable | ||
| menu Menu | ||
| Selected tracker.OptionalInt | ||
| TipArea component.TipArea | ||
| Tooltip component.Tooltip | ||
| } |
There was a problem hiding this comment.
the new "when putting MIDI into tracks, you can, optionally, select another track where the velocity byte goes" feature required a new type of button (that opens a popup menu)
| if b.Hidden { | ||
| return layout.Dimensions{} | ||
| } |
There was a problem hiding this comment.
also; I considered it better UX to make the MIDI button hidden if there is no input device available (this is checked once on startup, so if I plug a device in later, I need to restart the tracker - might still be better than checking all devices every single frame)
| if icon == nil { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Menu Items were forced to have an icon until now
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *widget.Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { | ||
| func (tr *Tracker) layoutMenu(gtx C, title string, clickable *Clickable, menu *Menu, width unit.Dp, items ...MenuItem) layout.Widget { |
There was a problem hiding this comment.
this corresponds to the change we introduced with the buttons-that-do-not-react-to-spacebar; we have our own Clickables which were not referenced consistently yet (probably still not everywhere, but progress is done in small steps ;) )
| in := layout.UniformInset(unit.Dp(1)) | ||
| voiceUpDown := func(gtx C) D { | ||
| numStyle := NumericUpDown(t.Theme, te.TrackVoices, "Number of voices for this track") | ||
| return in.Layout(gtx, numStyle.Layout) | ||
| } | ||
| voiceUpDown := NumericUpDownPadded(t.Theme, te.TrackVoices, "Number of voices for this track", 1) |
There was a problem hiding this comment.
ok, this was a change that I turned out not to need later on. improves readability imo
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA) { | ||
| func (te *NoteEditor) paintColumnCell(gtx C, x int, t *Tracker, c color.NRGBA, ignoreEffect bool) { | ||
| cw := gtx.Constraints.Min.X | ||
| cx := 0 | ||
| if t.Model.Notes().Effect(x) { | ||
| if t.Model.Notes().Effect(x) && !ignoreEffect { |
There was a problem hiding this comment.
because our MIDI input does some coloring of other tracks, it might need to ignore these other tracks "Effect" property (otherwise only one Nibble is colored)
|
Uh oh :D Another one of "23 files changed". While in principle I agree with the goal (getting note velocities), reviewing so many small refactorings on top of adding a new feature is a bit exhausting. I'll do my best, but I've just learned that I will be quite busy in work until January. But I'll try to squeeze in some time to review this. |
|
If there was some clear refactorings that needed to be done to make adding that feature easier, I'd appreciate if you can break these down into a bit more manageable chunks, like "refactor: X" "refactor: Y" and then "feat: Z". This was we can get the easy refactorings merged, while the new feature can take a bit more time and discussion. |
I am aware of the problem, and I know that this takes time to review. And it might very well be that some changes in there are not required for this issue (e.g. were part of some changes that turned out to be not the right path), but I wouldn't know how some of them are possibly separated into smaller requests (e.g. widget.Clickable -> Clickable, or the optional integer stuff). hope we'll manage. I'll go work on something else for now ;) |
|
dammit. didnt want to close. |
yes, I can see working on that |
|
Maybe you enjoy reviewing #183 more, this appears rather straightforward and might reduce the number of files to about a half :D |
…di/improve-midi-input # Conflicts: # tracker/gioui/tracker.go
Hi pestis,
this is probably large, but I do not know whether overly-large.
I noticed that my polyphonic midi input from a few weeks back (direct track input via my button, not the recording function) was not actually doing its thing as intended, and I wanted to add a way to also write the velocity of the midi input into another chosen track.
doing so included a lot of thoughts, discussion about structure with @LeStahL , and finally this solution which does its job, but might be a lot to review. anticipating your comments! 😬